Skip to content

Fix runtime bugs in model loading, singlepoint submission, and results views - #12

Merged
oerc0122 merged 1 commit into
stfc:mainfrom
jkshenton:ui-runtime-fixes
Oct 28, 2026
Merged

oerc0122 merged 1 commit into
stfc:mainfrom
jkshenton:ui-runtime-fixes

Conversation

@jkshenton

Copy link
Copy Markdown
Contributor

While running the app against ipywidgets 8 and testing calculation workflows, I hit several runtime errors that broke the UI or blocked submissions.

What broke and what changed

  1. Tab._titles removed in ipywidgets 8 (model.py)
    Selecting a model tab threw an AttributeError because ipywidgets 8 dropped the internal _titles dict. Switched to self.model_type.get_title(selected_index).

  2. Model cache path and code auto-selection (model.py, prediction.py)
    ModelData.from_uri used a relative cache_dir="mlips", writing cache files into whatever working directory the notebook kernel happened to be in. Changed both calls to Path.home() / ".cache" / "mlips". Also updated the code dropdown to automatically pick janus if it exists in the AiiDA profile, rather than whichever code sorted first.

  3. Singlepoint properties format (run.py)
    The singlepoint calculation builder expects properties as an AiiDA Str (comma-separated string), but was being passed an orm.List, which caused calculation submissions to fail.

  4. Handling in-progress or failed runs in results (results.py, node_viewers.py)
    In ResultsWizardStep, clicking on an active (waiting or running) calculation crashed because exit_status is None until the process finishes. Added an is_finished check before evaluating exit codes. In SummaryViewer, clicking a node that failed or was killed before writing results_dict caused a KeyError; it now prints the process state and exit status cleanly instead of crashing.

Testing

  • Verified model selection, singlepoint submission, and results view locally.
  • Test suite passes cleanly (pytest: 15 passed).

class SummaryViewer(ipw.VBox):
def __init__(self, node, **kwargs) -> None:

if not hasattr(node, "outputs") or "results_dict" not in node.outputs:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/aiidalab_mlip/steps/model.py Outdated

try:
model_str = ModelData.from_uri(model_uri, architecture=arch, cache_dir="mlips")
model_str = ModelData.from_uri(model_uri, architecture=arch, cache_dir=Path.home() / ".cache" / "mlips")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

def _try_load_model(self, arch: str) -> ModelData | None:

typ = self.model_type._titles[str(self.model_type.selected_index)]
typ = self.model_type.get_title(self.model_type.selected_index)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/aiidalab_mlip/steps/run.py Outdated
case "Single Point":
builder = Singlepoint.get_builder()
builder.properties = orm.List(list(task_parameters["properties"]))
props = task_parameters["properties"]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current master of aiida-mlip takes a aiida.orm.List (https://github.com/stfc/aiida-mlip/blob/main/aiida_mlip/calculations/singlepoint.py)

@alinelena

Copy link
Copy Markdown
Member

is this still blocking?

@jkshenton

Copy link
Copy Markdown
Contributor Author

I'm just rebasing now after the version and phonon branches were merged. I think those took care of most of the issues tbh. Let me do an actual build of the whole thing a see if there are lingering runtime issues


if code_labels:
self.code.value = code_labels[0]
if "janus" in code_labels:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very minor, but when you have 'feff' also installed the MLIP app defaults to that if this fix is not in place.


for pk, ctime, label, _state, exit_status in results:
status_icon = "OK" if exit_status == 0 else "ERR" if exit_status else "RUN"
if exit_status == 0:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need some guard like this, otherwise you get

Calculation exited with status: None

For things that are still running. There might be a neater fix though

@jkshenton

Copy link
Copy Markdown
Contributor Author

So the only two things left after rebasing are very minor and not blocking at all.

@oerc0122
oerc0122 merged commit feb2264 into stfc:main Oct 28, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants